net: ip: ipv6: replace legacy crypto with PSA API #97346
Conversation
valeriosetti
force-pushed
the
ipv6-replace-legacy-crypto
branch
from
cf232b7 to
6fd6589
Compare
valeriosetti
force-pushed
the
ipv6-replace-legacy-crypto
branch
from
6fd6589 to
094ab2a
Compare
|
Failing CI tests are due to Mbed TLS not properly sizing the internal buffer for PSA keys ( |
|
The following west manifest projects have changed revision in this Pull Request:
⛔ DNM label due to: 1 project with PR revision Note: This message is automatically posted and updated by the Manifest GitHub Action. |
github-actions
bot
added
manifest
manifest-mbedtls
DNM (manifest)
| path: modules/lib/gui/lvgl | ||
| - name: mbedtls | ||
| revision: 85440ef5fffa95d0e9971e9163719189cf34d979 | ||
| revision: pull/76/head |
There was a problem hiding this comment.
Would be nice to know what you're pulling in the commit title (e.g. manifest: mbedtls: pull X fix) + typo: "alloacated"
There was a problem hiding this comment.
pull PR 76 doesn't really give out information 🙂 One can easily trace back what PR the commit came from. I rather meant something like pull fix for [...], but anyway you'll be updating this commit later when the PR is merged.
| select MBEDTLS | ||
| select MBEDTLS_MD | ||
| select MBEDTLS_PSA_CRYPTO_C |
There was a problem hiding this comment.
I know #96415 is not merged yet, but are you planning to update this (and similar occurrences)?
There was a problem hiding this comment.
Yes, #96415 will likely be merged before this one. Once this will be done I will update this PR accordingly. I didn't do this for now because I don't want spurious failures in the CI.
| psa_set_key_type(&key_attr, PSA_KEY_TYPE_HMAC); | ||
| psa_set_key_algorithm(&key_attr, PSA_ALG_HMAC(PSA_ALG_SHA_256)); | ||
| psa_set_key_usage_flags(&key_attr, PSA_KEY_USAGE_SIGN_MESSAGE); | ||
| status = psa_import_key(&key_attr, secret_key, sizeof(secret_key), &key_id); |
There was a problem hiding this comment.
Wondering if instead of importing the key every time we could just have the key ID as static in this function and import or generate it just once directly in PSA Crypto. 🤔
There was a problem hiding this comment.
When should we then destroy that key? The drawback I see in doing what you are suggesting is that this key will occupy a slot in PSA core until is destroyed or the device is reset.
How often are IPv6 addresses supposed to change (with/without privacy extension)? Unless this happens quite often, having a key slot always occupied might require a larger number of key slots in the PSA core.
I'm not against the requested change, but I would like to know if you have any strong preference on this footprint/performance compromise.
There was a problem hiding this comment.
valeriosetti
force-pushed
the
ipv6-replace-legacy-crypto
branch
from
c723984 to
8759dc8
Compare
zephyrbot
added
the
area: Tests
Zephyr's long term goal for crypto support is to use only PSA API. This commit replaces usages of MD with proper PSA API for HMAC. Signed-off-by: Valerio Setti <[email protected]>
Moving from legacy Mbed TLS crypto to PSA API caused mps2/an385 platform to fail due to stack overflow. This commit increases the stack size to solve this problem. Signed-off-by: Valerio Setti <[email protected]>
Include a fix for the allocated buffers in case of static key slots. This commit will be updated once the corresponding PR is merged. Signed-off-by: Valerio Setti <[email protected]>
valeriosetti
force-pushed
the
ipv6-replace-legacy-crypto
branch
from
8759dc8 to
ed7a555
Compare
|
There was a problem hiding this comment.
Looks good for now, pending merging of #96415 and zephyrproject-rtos/mbedtls#76.
5 participants
Zephyr's long term goal for crypto support is to use only PSA API. This PR replaces usages of MD with proper PSA API for HMAC.